Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opensearch - Remove CurrentState attribute #2162

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ichekaldin
Copy link
Contributor

SUMMARY

This is necessary because describe_domain_config method returns both CurrentState and DesiredState while update_domain_config method only expects DesiredState.

Fixes #2161.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

opensearch

ADDITIONAL INFORMATION

This is necessary because describe_domain_config method returns both CurrentState and DesiredState
while update_domain_config method only expects DesiredState.
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/583c9489b40945f6b4c9e0ce8dcfce69

ansible-galaxy-importer FAILURE in 5m 21s (non-voting)
✔️ build-ansible-collection SUCCESS in 11m 15s
✔️ ansible-test-splitter SUCCESS in 4m 23s
Skipped 22 jobs

@markuman
Copy link
Member

Can you also please add an integration test that covers this case? https://github.com/ansible-collections/community.aws/tree/main/tests/integration/targets/opensearch

Otherwise LGTM

@ichekaldin
Copy link
Contributor Author

@markuman, as far as I can tell, integration tests for opensearch are disabled.

I'm fairly certain the existing tests would catch it by failing.

I'm not exactly sure when this piece of Opensearch functionality was introduced. I suspect it happened between April 25, 2024 (which is the most recent successful build of my Ansible automation that touched Opensearch) and now.

@markuman
Copy link
Member

markuman commented Oct 1, 2024

@markuman, as far as I can tell, integration tests for opensearch are disabled.

Ah ok. I'll run the opensearch test locally later this day and give you a feedback.

@ichekaldin
Copy link
Contributor Author

@markuman, any luck with the testing?

@ichekaldin
Copy link
Contributor Author

@markuman, any luck with this?

@markuman
Copy link
Member

markuman commented Jan 3, 2025

The current integration test does not work anymore and need more care.

diff --git a/tests/integration/targets/opensearch/defaults/main.yml b/tests/integration/targets/opensearch/defaults/main.yml
index da6aef4b..03a5e04f 100644
--- a/tests/integration/targets/opensearch/defaults/main.yml
+++ b/tests/integration/targets/opensearch/defaults/main.yml
@@ -1,2 +1,5 @@
 ---
 # defaults file for opensearch tests
+min_engine_version: OpenSearch_1.3
+default_engine_version: OpenSearch_2.17
+tls_version: Policy-Min-TLS-1-2-2019-07
diff --git a/tests/integration/targets/opensearch/tasks/test_opensearch.yml b/tests/integration/targets/opensearch/tasks/test_opensearch.yml
index 7ce1f8d9..42466155 100644
--- a/tests/integration/targets/opensearch/tasks/test_opensearch.yml
+++ b/tests/integration/targets/opensearch/tasks/test_opensearch.yml
@@ -2,7 +2,7 @@
   block:
     - name: test without specifying required module options
       opensearch:
-        engine_version: "Elasticsearch_7.1"
+        engine_version: "{{ min_engine_version }}"
       ignore_errors: true
       register: result
 
@@ -15,7 +15,7 @@
       opensearch:
         state: present
         domain_name: "es-{{ tiny_prefix }}-public"
-        engine_version: "OpenSearch_1.1"
+        engine_version: "{{ default_engine_version }}""
         cluster_config:
           instance_type: "t2.small.search"
           instance_count: 2
@@ -41,7 +41,7 @@
         state: present
         # Note domain_name must be less than 28 characters and satisfy regex [a-z][a-z0-9\\-]+
         domain_name: "es-{{ tiny_prefix }}-public"
-        engine_version: "OpenSearch_1.1"
+        engine_version: "{{ default_engine_version }}""
         cluster_config:
           instance_type: "t2.small.search"
           instance_count: 2
@@ -66,7 +66,7 @@
     - assert:
         that:
           - "opensearch_domain.tags | length == 4"
-          - "opensearch_domain.engine_version == 'OpenSearch_1.1'"
+          - "opensearch_domain.engine_version == default_engine_version"
           - "opensearch_domain.cluster_config.instance_count == 2"
           - "opensearch_domain.cluster_config.instance_type == 't2.small.search'"
           - "opensearch_domain.cluster_config.dedicated_master_enabled == false"
@@ -88,7 +88,7 @@
       opensearch:
         state: present
         domain_name: "es-{{ tiny_prefix }}-public"
-        engine_version: "OpenSearch_1.1"
+        engine_version: "{{ default_engine_version }}""
         cluster_config:
           instance_type: "t2.small.search"
           instance_count: 2
@@ -112,7 +112,7 @@
       opensearch:
         state: present
         domain_name: "es-{{ tiny_prefix }}-public"
-        engine_version: "OpenSearch_1.1"
+        engine_version: "{{ default_engine_version }}""
         cluster_config:
           instance_type: "t2.small.search"
           instance_count: 2
@@ -135,7 +135,7 @@
       opensearch:
         state: present
         domain_name: "es-{{ tiny_prefix }}-vpc"
-        engine_version: "Elasticsearch_7.1"
+        engine_version: "{{ min_engine_version }}"
         cluster_config:
           instance_type: "m5.large.search"
           instance_count: 2
@@ -179,7 +179,7 @@
       opensearch:
         state: present
         domain_name: "es-{{ tiny_prefix }}-vpc"
-        engine_version: "Elasticsearch_7.1"
+        engine_version: "{{ min_engine_version }}"
         cluster_config:
           instance_type: "m5.large.search"
           instance_count: 2
@@ -241,7 +241,7 @@
       opensearch:
         state: present
         domain_name: "es-{{ tiny_prefix }}-vpc"
-        engine_version: "Elasticsearch_7.1"
+        engine_version: "{{ min_engine_version }}"
         cluster_config:
           instance_type: "m5.large.search"
           instance_count: 2
@@ -281,7 +281,7 @@
       opensearch:
         state: present
         domain_name: "es-{{ tiny_prefix }}-vpc"
-        engine_version: "Elasticsearch_7.1"
+        engine_version: "{{ min_engine_version }}"
         cluster_config:
           instance_type: "m5.large.search"
           instance_count: 2
@@ -932,7 +932,7 @@
         domain_name: "es-{{ tiny_prefix }}-vpc"
         domain_endpoint_options:
           enforce_https: true
-          tls_security_policy: "Policy-Min-TLS-1-0-2019-07"
+          tls_security_policy: "{{ tls_version }}"
         wait: true
       check_mode: true
       register: opensearch_domain
@@ -945,7 +945,7 @@
         domain_name: "es-{{ tiny_prefix }}-vpc"
         domain_endpoint_options:
           enforce_https: true
-          tls_security_policy: "Policy-Min-TLS-1-0-2019-07"
+          tls_security_policy: "{{ tls_version }}"
           # Refer to CNAME that was defined in the previous tasks.
           custom_endpoint_enabled: true
           custom_endpoint: "opensearch.ansible-integ-test.com"
@@ -966,7 +966,7 @@
     - assert:
         that:
           - "opensearch_domain.domain_endpoint_options.enforce_https == True"
-          - "opensearch_domain.domain_endpoint_options.tls_security_policy == 'Policy-Min-TLS-1-0-2019-07'"
+          - "opensearch_domain.domain_endpoint_options.tls_security_policy == tls_version"
           #- "opensearch_domain.domain_endpoint_options.custom_endpoint_enabled == True"
           - opensearch_domain is changed

However, the integration test is still failing at: tests/integration/targets/opensearch/tasks/test_vpc_setup.yml:121:

TASK [opensearch : Create KMS key for test purpose] ****************************
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.errorfactory.MalformedPolicyDocumentException: An error occurred (MalformedPolicyDocumentException) when calling the CreateKey operation: Policy contains a statement with one or more invalid principals.
fatal: [testhost]: FAILED! => {"boto3_version": "1.26.0", "botocore_version": "1.29.0", "changed": false, "error": {"code": "MalformedPolicyDocumentException", "message": "Policy contains a statement with one or more invalid principals."}, "message": "Policy contains a statement with one or more invalid principals.", "msg": "Failed to create initial key: An error occurred (MalformedPolicyDocumentException) when calling the CreateKey operation: Policy contains a statement with one or more invalid principals.", "resource_actions": ["kms:CreateKey", "kms:DescribeKey"], "response_metadata": {"http_headers": {"cache-control": "no-cache, no-store, must-revalidate, private", "connection": "keep-alive", "content-length": "122", "content-type": "application/x-amz-json-1.1", "date": "Fri, 03 Jan 2025 11:12:58 GMT", "expires": "0", "pragma": "no-cache", "x-amzn-requestid": "b82e3ad5-219e-418e-8a11-b5b742a0c077"}, "http_status_code": 400, "request_id": "b82e3ad5-219e-418e-8a11-b5b742a0c077", "retry_attempts": 0}}

So some of the principals defined here must be wrong: https://github.com/ansible-collections/community.aws/blob/main/tests/integration/targets/opensearch/templates/kms_policy.j2
ATM I don't see which one is faulty. Any ideas @ichekaldin ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opensearch Module Fails to Update Existing Domains
2 participants